Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

For Mobilenet V4 large #821

Merged
merged 31 commits into from
Feb 27, 2024
Merged

For Mobilenet V4 large #821

merged 31 commits into from
Feb 27, 2024

Conversation

freedomtan
Copy link
Contributor

@freedomtan freedomtan commented Nov 28, 2023

Use Mobilenet V4 large for Image Classification V2.

Copy link

github-actions bot commented Nov 28, 2023

MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅

Copy link

sonarcloud bot commented Nov 28, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 5 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@Mostelk Mostelk marked this pull request as ready for review November 29, 2023 04:48
@Mostelk Mostelk requested review from anhappdev and a team as code owners November 29, 2023 04:48
@freedomtan freedomtan marked this pull request as draft December 4, 2023 03:08
@freedomtan
Copy link
Contributor Author

@mohitmundhragithub I fixed a stupid bug for floating model. Please try again.

@mohitmundhragithub
Copy link
Contributor

mohitmundhragithub commented Jan 8, 2024

@mohitmundhragithub I fixed a stupid bug for floating model. Please try again.

Thanks a lot @freedomtan to share this.

I have added one more commit to modify the input images required for Calibration set for PTQ. Please have look.
@Mostelk, IMHO this removes the need for the default normalization model, which you have asked from Colby over email.

@freedomtan
Copy link
Contributor Author

freedomtan commented Feb 1, 2024

@anhappdev For MobileNet EdgeTPU, the input tensor is 224x224x3; for newly introduced model, it's 384x384x3. What's the best way to have both them? Add new dataset type, e.g., imagenet_384? Or, we should add width and height paramters?

@anhappdev
Copy link
Collaborator

anhappdev commented Feb 1, 2024

@anhappdev For MobileNet EdgeTPU, the input tensor is 224x224x3; for newly introduced model, it's 384x384x3. What's the best way to have both them? Add new dataset type, e.g., imagenet_384? Or, we should add width and height paramters?

Where should we have both of them? Do you mean the image_classification_v2 task should have multiple models with different input sizes?

There is already an image_width and image_height parameter in ModelConfig.

message ModelConfig {
// unique ID used by UI to find matching description
required string id = 1;
// Human-readable name of the model.
required string name = 2;
// Offset value of the model if applicable.
optional int32 offset = 3 [default = 0];
// image width of the input (if model takes images)
optional int32 image_width = 4;
// image height of the input (if model takes images)
optional int32 image_height = 5;
// Number of detection classes if applicable
optional int32 num_classes = 6;
}

I

@freedomtan
Copy link
Contributor Author

@anhappdev For MobileNet EdgeTPU, the input tensor is 224x224x3; for newly introduced model, it's 384x384x3. What's the best way to have both them? Add new dataset type, e.g., imagenet_384? Or, we should add width and height paramters?

Where should we have both of them? Do you mean the image_classification_v2 task should have multiple models with different input sizes?

nope, image_classification and image_classification_v2 have different sizes.

There is already an image_width and image_height parameter in ModelConfig.

message ModelConfig {
// unique ID used by UI to find matching description
required string id = 1;
// Human-readable name of the model.
required string name = 2;
// Offset value of the model if applicable.
optional int32 offset = 3 [default = 0];
// image width of the input (if model takes images)
optional int32 image_width = 4;
// image height of the input (if model takes images)
optional int32 image_height = 5;
// Number of detection classes if applicable
optional int32 num_classes = 6;
}

I

We hard-coded image size for imagenet dataset at https://github.com/mlcommons/mobile_app_open/blob/master/flutter/cpp/flutter/dart_run_benchmark.cc#L73-L77

  switch (in->dataset_type) {
    case ::mlperf::mobile::DatasetConfig::IMAGENET:
      dataset = std::make_unique<::mlperf::mobile::Imagenet>(
          backend.get(), in->dataset_data_path, in->dataset_groundtruth_path,
          in->dataset_offset, 224, 224 /* width, height */);
      break;
...

@anhappdev
Copy link
Collaborator

We hard-coded image size for imagenet dataset at ...

In that case, we just need to update the code to read the image size from the task file?

@freedomtan
Copy link
Contributor Author

freedomtan commented Feb 2, 2024

We hard-coded image size for imagenet dataset at ...

In that case, we just need to update the code to read the image size from the task file?

Yes, that's a better solution. A dirty hack is to add something like case ::mlperf::mobile::DatasetConfig::IMAGENET_384:

  switch (in->dataset_type) {
    case ::mlperf::mobile::DatasetConfig::IMAGENET:
      dataset = std::make_unique<::mlperf::mobile::Imagenet>(
          backend.get(), in->dataset_data_path, in->dataset_groundtruth_path,
          in->dataset_offset, 224, 224 /* width, height */);
      break;

How to update our code to pass width and height to the dart_ffi_run_benchmark_out* dart_ffi_run_benchmark in
https://github.com/mlcommons/mobile_app_open/blob/master/flutter/cpp/flutter/dart_run_benchmark.cc

@anhappdev
Copy link
Collaborator

Yes, that's a better solution.
How to update our code to pass width and height to the dart_ffi_run_benchmark_out* dart_ffi_run_benchmark in
https://github.com/mlcommons/mobile_app_open/blob/master/flutter/cpp/flutter/dart_run_benchmark.cc

I can send a PR to pass image_width and image_height to dart_ffi_run_benchmark. It requires code changes in multiple places.

@anhappdev
Copy link
Collaborator

@freedomtan Can you rebase this PR on the 719/new-edge-tpu-model and also change this PR to merge into 719/new-edge-tpu-model instead of master? I just updated 719/new-edge-tpu-model with the latest commits from master.

@freedomtan
Copy link
Contributor Author

@freedomtan Can you rebase this PR on the 719/new-edge-tpu-model and also change this PR to merge into 719/new-edge-tpu-model instead of master? I just updated 719/new-edge-tpu-model with the latest commits from master.

sure, I'll do it.

@anhappdev anhappdev changed the base branch from master to 719/new-edge-tpu-model February 2, 2024 06:33
@anhappdev
Copy link
Collaborator

@freedomtan
Copy link
Contributor Author

freedomtan commented Feb 13, 2024

@RSMNYS The CoreML model for MobilenetV4 Large converted TensorFlow doesn't perform as good as as expected, this could be used a case for optimizing CoreML models.

@freedomtan
Copy link
Contributor Author

@anhappdev Any comments of how to the 6 failed tests?

@freedomtan
Copy link
Contributor Author

freedomtan commented Feb 13, 2024

  • Pixel backend: Tested on @freedomtan's Pixel 7a
  • tflite fallback: tested on a Pixel 4 by @freedomtan
  • MTK Neuron
  • iOS CoreML

@anhappdev
Copy link
Collaborator

@anhappdev Any comments of how to the 6 failed tests?

They mostly failed on expected accuracy or throughput. I will run them several times and update the expected values.

@anhappdev
Copy link
Collaborator

Currently, the test-android-apk-tflite and test-android-apk-pixel failed because accuracy is always 0.0.
I don't have a Pixel device to debug this. @freedomtan Can you take a look and run the test locally (with make flutter/test cmd) with your devices to test?

* Update S3 provider in android-build-test.yml

* Update env var name

---------

Co-authored-by: Anh <[email protected]>
@freedomtan
Copy link
Contributor Author

freedomtan commented Feb 19, 2024

flutter/test

sometimes it failed, sometimes all tests passed

tested on Pixel 7 Pro running latest Android 14.

an "all tests passed" example was uploaded to Firebase,

Uploaded to user/B0rnrmYyh9XMldU5CZuzRELFZRY2/result/2024-02-20T06-21-02_9b560a3c-41b8-40f5-b7b7-6f8526ee61e0.json

@freedomtan
Copy link
Contributor Author

Let's test the TFLite backend on more devices to see if we can figure out why we got accuracy equals 0 on Pixel 5.

@freedomtan
Copy link
Contributor Author

On a Samsung Galaxy S22+ (Exynos 2200), with TFLite backend only, I got expected accuracy numbers.

@anhappdev
Copy link
Collaborator

anhappdev commented Feb 20, 2024

Pixel backend:

  • image_classification_v2:

    • Pixel 6, API Level 33 > accuracy OK
    • Pixel 6, API Level 32 > accuracy OK
    • Pixel 6, API Level 31 > accuracy OK
  • image_classification_offline_v2:

    • Pixel 6, API Level 33 > accuracy Zero
    • Pixel 6, API Level 32 > accuracy Zero
    • Pixel 6, API Level 31 > crash

TFLite backend:

  • image_classification_v2:

    • Pixel 6, API Level 33 > accuracy OK
    • Pixel 6, API Level 32 > accuracy OK
    • Pixel 6, API Level 31 > accuracy OK
    • Pixel 5, API Level 30 > accuracy Zero
  • image_classification_offline_v2:

    • Pixel 6, API Level 33 > accuracy OK
    • Pixel 6, API Level 32 > accuracy OK
    • Pixel 6, API Level 31 > accuracy OK
    • Pixel 5, API Level 30 > accuracy Zero

@freedomtan
Copy link
Contributor Author

Pixel backend:

  • image_classification_v2:

    • Pixel 6, API Level 33 > accuracy OK
    • Pixel 6, API Level 32 > accuracy OK
    • Pixel 6, API Level 31 > accuracy OK
  • image_classification_offline_v2:

    • Pixel 6, API Level 33 > accuracy Zero
    • Pixel 6, API Level 32 > accuracy Zero
    • Pixel 6, API Level 31 > crash

TFLite backend:

  • image_classification_v2:

    • Pixel 6, API Level 33 > accuracy OK
    • Pixel 6, API Level 32 > accuracy OK
    • Pixel 6, API Level 31 > accuracy OK
    • Pixel 5, API Level 30 > accuracy Zero
  • image_classification_offline_v2:

    • Pixel 6, API Level 33 > accuracy OK
    • Pixel 6, API Level 32 > accuracy OK
    • Pixel 6, API Level 31 > accuracy OK
    • Pixel 5, API Level 30 > accuracy Zero

That's interesting. I guess for Pixel backend + image_classification_offline_v2, we can get expected accuracy by reducing batch size.

For Pixel 5, either API level or other software stack issues?

@anhappdev
Copy link
Collaborator

For Pixel 5, either API level or other software stack issues?

The Pixel 5 on Firebase Test Lab has only API Level 30 available, so I cannot test it with other API levels.
I tested on Motorola G20, API Level 30, TFLite backend > the accuracy is OK.
So I guess the issue is something specific to Pixel 5.

Copy link

sonarcloud bot commented Feb 21, 2024

@RSMNYS
Copy link
Contributor

RSMNYS commented Feb 22, 2024

@RSMNYS The CoreML model for MobilenetV4 Large converted TensorFlow doesn't performance as good as as expected, this could be used a case for optimizing CoreML models.

@freedomtan is this model the one we need to work with: https://github.com/mlcommons/mobile_open/raw/main/vision/mobilenetV4?

or this savedModel: https://github.com/mlcommons/mobile_open/releases/tag/model_upload?

@freedomtan
Copy link
Contributor Author

@RSMNYS The CoreML model for MobilenetV4 Large converted TensorFlow doesn't performance as good as as expected, this could be used a case for optimizing CoreML models.

@freedomtan is this model the one we need to work with: https://github.com/mlcommons/mobile_open/raw/main/vision/mobilenetV4?

or this savedModel: https://github.com/mlcommons/mobile_open/releases/tag/model_upload?

Either one, or say, neither of them. We convert from the saved_model to CoreML model. The performance of the converted CoreML model is not as good as expected. Please check how we can get a better CoreML model from either tflite of saved_model.

@anhappdev anhappdev marked this pull request as ready for review February 27, 2024 06:19
@anhappdev anhappdev merged commit c128476 into 719/new-edge-tpu-model Feb 27, 2024
19 of 21 checks passed
@anhappdev anhappdev deleted the test_new_model_l branch February 27, 2024 06:57
@github-actions github-actions bot locked and limited conversation to collaborators Feb 27, 2024
@RSMNYS
Copy link
Contributor

RSMNYS commented Mar 4, 2024

@RSMNYS The CoreML model for MobilenetV4 Large converted TensorFlow doesn't performance as good as as expected, this could be used a case for optimizing CoreML models.

@freedomtan is this model the one we need to work with: https://github.com/mlcommons/mobile_open/raw/main/vision/mobilenetV4?
or this savedModel: https://github.com/mlcommons/mobile_open/releases/tag/model_upload?

Either one, or say, neither of them. We convert from the saved_model to CoreML model. The performance of the converted CoreML model is not as good as expected. Please check how we can get a better CoreML model from either tflite of saved_model.

@freedomtan are we expecting to have the MobilenetV4 performance on the task image_classification_V2 similar as for the https://github.com/mlcommons/mobile_models/raw/main/v3_0/CoreML/MobilenetEdgeTPU.mlmodel which Is used for the image_classification task (currently near 1000 qps)?

@freedomtan
Copy link
Contributor Author

@RSMNYS The CoreML model for MobilenetV4 Large converted TensorFlow doesn't performance as good as as expected, this could be used a case for optimizing CoreML models.

@freedomtan is this model the one we need to work with: https://github.com/mlcommons/mobile_open/raw/main/vision/mobilenetV4?
or this savedModel: https://github.com/mlcommons/mobile_open/releases/tag/model_upload?

Either one, or say, neither of them. We convert from the saved_model to CoreML model. The performance of the converted CoreML model is not as good as expected. Please check how we can get a better CoreML model from either tflite of saved_model.

@freedomtan are we expecting to have the MobilenetV4 performance on the task image_classification_V2 similar as for the https://github.com/mlcommons/mobile_models/raw/main/v3_0/CoreML/MobilenetEdgeTPU.mlmodel which Is used for the image_classification task (currently near 1000 qps)?

as far as I can tell, 1000 qps is too good to be true for Mobilenet V4 Large. But 300 qps should be possible, I guess.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants